Skip to content

Exception in JUnit test after disabling JVM forking #112

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Nov 3, 2016
Merged

Exception in JUnit test after disabling JVM forking #112

merged 1 commit into from
Nov 3, 2016

Conversation

ashawley
Copy link
Member

@ashawley ashawley commented Oct 4, 2016

Disabling JVM forking while working on #110 causes a JUnit test to fail.

$ sbt
> testOnly scala.xml.pull.XMLEventReaderTest

This unit test hasn't been an issue in the past, but was found while trying to troubleshoot a different Travis build error after adding a Scalacheck test suite to scala-xml. It's not clear if this is the actual issue that adding Scalacheck introduce. Very likely, it could just be a false lead after desperately trying to understand the original problem.

@ashawley ashawley force-pushed the disable-test-forking branch from 5b5a38e to 1b91ef7 Compare October 18, 2016 15:45
ashawley
ashawley commented Oct 23, 2016
View reviewed changes
src/main/scala/scala/xml/parsing/MarkupParserCommon.scala
@@ -250,7 +250,6 @@ private[scala] trait MarkupParserCommon extends TokenTests {
else if (ch == SU || eof)
truncatedError("") // throws TruncatedXMLControl in compiler

sb append ch
Copy link
Member Author

@ashawley ashawley Oct 23, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the line in scala-xml that in turn causes the OOM exception to be thrown by java.util.Arrays by way of java.lang.StringBuilder. Even after removing the line in question from scala-xml, see above, it still causes the test suite to run out of memory. Which scala-xml is it running that causes the memory overflow? It doesn't seem to be the code under test.

@ashawley
Copy link
Member Author

Removing the line that emits the exception, still causes the exception.

This can be verified from the Travis build:

Exception in thread "XMLEventReader" java.lang.OutOfMemoryError: Java heap space
    at java.util.Arrays.copyOf(Arrays.java:3332)
    at java.lang.AbstractStringBuilder.expandCapacity(AbstractStringBuilder.java:137)
    at java.lang.AbstractStringBuilder.ensureCapacityInternal(AbstractStringBuilder.java:121)
    at java.lang.AbstractStringBuilder.append(AbstractStringBuilder.java:622)
    at java.lang.StringBuilder.append(StringBuilder.java:202)
    at scala.collection.mutable.StringBuilder.append(StringBuilder.scala:267)
    at scala.xml.parsing.MarkupParserCommon.xTakeUntil(MarkupParserCommon.scala:253)
    at scala.xml.parsing.MarkupParserCommon.xTakeUntil$(MarkupParserCommon.scala:238)
    at scala.xml.pull.XMLEventReader$Parser.xTakeUntil(XMLEventReader.scala:60)
    at scala.xml.parsing.MarkupParser.xCharData(MarkupParser.scala:379)
    at scala.xml.parsing.MarkupParser.xCharData$(MarkupParser.scala:373)
    at scala.xml.pull.XMLEventReader$Parser.xCharData(XMLEventReader.scala:60)
    at scala.xml.parsing.MarkupParser.content1(MarkupParser.scala:424)
    at scala.xml.parsing.MarkupParser.content1$(MarkupParser.scala:419)
    at scala.xml.pull.XMLEventReader$Parser.content1(XMLEventReader.scala:60)
    at scala.xml.parsing.MarkupParser.content(MarkupParser.scala:459)
    at scala.xml.parsing.MarkupParser.content$(MarkupParser.scala:442)
    at scala.xml.pull.XMLEventReader$Parser.content(XMLEventReader.scala:60)
    at scala.xml.parsing.MarkupParser.element1(MarkupParser.scala:588)
    at scala.xml.parsing.MarkupParser.element1$(MarkupParser.scala:573)
    at scala.xml.pull.XMLEventReader$Parser.element1(XMLEventReader.scala:60)
    at scala.xml.parsing.MarkupParser.content1(MarkupParser.scala:433)
    at scala.xml.parsing.MarkupParser.content1$(MarkupParser.scala:419)
    at scala.xml.pull.XMLEventReader$Parser.content1(XMLEventReader.scala:60)
    at scala.xml.parsing.MarkupParser.document(MarkupParser.scala:247)
    at scala.xml.parsing.MarkupParser.document$(MarkupParser.scala:225)
    at scala.xml.pull.XMLEventReader$Parser.document(XMLEventReader.scala:60)
    at scala.xml.pull.XMLEventReader$Parser.$anonfun$run$1(XMLEventReader.scala:96)
    at scala.xml.pull.XMLEventReader$Parser$$Lambda$14/510432178.apply(Unknown Source)
    at scala.xml.pull.ProducerConsumerIterator.interruptibly(XMLEventReader.scala:125)
    at scala.xml.pull.ProducerConsumerIterator.interruptibly$(XMLEventReader.scala:125)
    at scala.xml.pull.XMLEventReader.interruptibly(XMLEventReader.scala:27)

@SethTisue
Copy link
Member

this is kind of a wild guess, but XMLEventReader uses a background thread. perhaps that thread needs to be explicitly shut down?

@ashawley
Copy link
Member Author

Thanks for pointing that out. That's curious. I hadn't noticed it.

I'll try calling stop in the unit tests.

@ashawley
Copy link
Member Author

Unfortunately, no joy after adding stop to the tests at least.

@gourlaysama
Copy link

You cannot disable JVM test forking when working on a scala module that is distributed with the compiler (which is the case for scala-xml and scala-parser-combinators) because of an SBT classloader leaking issue (#20).

Basically, when not forking, the test are compiled against the locally built classes, but they are run against the ones bundled with the compiler (the ones from the bootclasspath, or from the special scala-toolconfiguration that contains the compiler and its dependencies).

@ashawley
Copy link
Member Author

@gourlaysama Thanks for this. I was losing my mind understanding what was amiss.

I had seen @jsuereth branch, wip/fix-build-classpath, but didn't know what it was about until you mentioned the issue it was associated with.

@ashawley
Copy link
Member Author

I've reworked this PR to add an explicit setting of fork in the build.sbt file, and added @gourlaysama's words and mentioned both this issue and the original one.

@ashawley ashawley force-pushed the disable-test-forking branch from f11f8a8 to 887fbe1 Compare October 27, 2016 18:56
@ashawley
Document why JVM forking has to be used in SBT
150a237 
Add a comment with a description of the issue by Antoine Gourlay and
refer to the relevant GitHub issues.
@ashawley ashawley force-pushed the disable-test-forking branch from 887fbe1 to 150a237 Compare October 27, 2016 18:57
@SethTisue
Copy link
Member

LGTM

@ashawley ashawley mentioned this pull request Oct 27, 2016
Closed
@SethTisue
Copy link
Member

final review/merge by @biswanaths ?

@biswanaths biswanaths merged commit 150a237 into scala:master Nov 3, 2016
@biswanaths
Copy link
Contributor

@SethTisue words are final though.

Thank you @SethTisue for review and @ashawley for the PR.

@ashawley ashawley deleted the disable-test-forking branch November 11, 2016 06:36
@ashawley ashawley mentioned this pull request Dec 1, 2016
Closed
ashawley added a commit to ashawley/scala-xml that referenced this pull request Dec 17, 2016
@ashawley
Undo JVM forking in test, reverting scala#112
99e5200 
* build.sbt (fork): SBT was leaking scala-xml from scala-compiler
because of an incorrect library dependency exclude rule.
@ashawley ashawley mentioned this pull request Dec 17, 2016
Merged
ashawley added a commit to ashawley/scala-xml that referenced this pull request Dec 25, 2016
@ashawley
Undo JVM forking in test, reverting scala#112
2920005 
* build.sbt (fork): SBT was leaking scala-xml from scala-compiler
because of an incorrect library dependency exclude rule.
ashawley added a commit to ashawley/scala-xml that referenced this pull request Feb 7, 2017
@ashawley
Undo JVM forking in test, reverting scala#112
afeefa0 
* build.sbt (fork): SBT was leaking scala-xml from scala-compiler
because of an incorrect library dependency exclude rule.
ashawley added a commit to ashawley/scala-xml that referenced this pull request Feb 15, 2017
@ashawley
Undo JVM forking in test, reverting scala#112
4f30084 
* build.sbt (fork): SBT was leaking scala-xml from scala-compiler
because of an incorrect library dependency exclude rule.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ashawley @SethTisue @gourlaysama @biswanaths